Skip to content

sleep: wire the HR-only spine, on a percentile anchor rather than the median - #1806

Merged
ryanbr merged 5 commits into
mainfrom
feat/1801-wire-hr-only-spine
Sep 2, 2026
Merged

ryanbr merged 5 commits into
mainfrom
feat/1801-wire-hr-only-spine

Conversation

@ryanbr

@ryanbr ryanbr commented Sep 2, 2026

Copy link
Copy Markdown
Owner

Refs #1801. Wires hrOnlySessions into the day scan for a strap with no motion — and fixes a real
flaw in the spine merged in #1805 that writing the tests exposed.

The anchor was wrong, by arithmetic

hrOnlySleepRuns took its band from hrBaseline, the window median, on the reasoning that
reusing the motion path's definition was parity. That was wrong, and not as a matter of tuning:

A median splits the samples in half by definition, so a band of median × 1.05 admits strictly
more than half of any window, whatever the data.

Measured on a realistic 24 h — 16 h awake at 74–96, 8 h night at 59–70 — it called 14.4 hours
sleep against a truth of 8
.

hrSleepBandMult is correct where it was designed to be used: a confirmation gate on a run
gravity stillness already found, where permissiveness is deliberate. A confirmation threshold and a
detection threshold are not the same instrument, and #1805's doc argued they were.

Now anchored on hrOnlyBaseline — the tenth percentile, nearest rank — which sits in the
night's trough and cannot admit half a window however the day is shaped.

Why p10 × 1.05

Swept anchor × multiplier against windows with known truth, taking the conservative corner
because the failure directions are not symmetric: over-detection puts a wrong Rest on screen,
under-detection leaves "No data", which is the state this feature improves on.

anchor mult field-like 24 h 54 h, two nights flat sleeper
median (#1805) 1.05 14.4 / 8 — —
p10 1.05 8.1 / 8 8.7 / 16 5.9 / 8
p05 1.10 9.5 / 8 8.7 / 16 8.0 / 8
p15 1.05 10.8 / 8 11.7 / 16 8.0 / 8

hrOnlyBandMult is a separate constant from hrSleepBandMult — equal today, but a detector's
tolerance and a confirmation gate's have no business moving together.

Merge the runs, or the night never survives

A run boundary is a threshold crossing, and a sleeping heart rate oscillates across the band all
night. Without mergePeriods the spine returned the night's minutes correctly but shredded into
sub-mergeMin fragments, every one of which then failed the minimum-duration gate — 8 h of
detected sleep yielding zero sessions
. The motion path calls mergePeriods for exactly this
reason; now so does this one.

Display-only, enforced structurally

An HR-only night carries null restingHR and avgHRV, and the AnalyticsEngine enrichment
that would refill them now skips it. Those two values are what Charge and the baselines fold in, and
a baseline is the one thing a false positive cannot be unwound from. Withholding them is structural;
a downstream filter would be one forgotten call site from failing open.

The Swift enrichment also now names hrOnly when it rebuilds the struct — unlike Kotlin's copy,
that rebuild drops a new field by default.

A WHOOP 4.0 is untouched

Reported working, and it must stay that way: the fallback sits behind grav.size < 2 and only
when the device persisted no hypnogram of its own, so a strap that banks motion keeps the motion
spine. A working detector must not be replaced by a weaker one. Pinned by a source-reading test.

Two comments this change falsified, corrected

  • the no-motion diagnostic no longer claims the stager has no HR-only fallback;
  • the Apple day-cache no longer justifies itself on "a WHOOP always streams gravity" (it now notes
    the HR-only providedSleep derives from the very HR the cache key already fingerprints).

Verification

  • 18 Kotlin tests, 0 failures across the three HR-only classes; result XML checked for real
    counts, not a zero-match filter.
  • Swift twins compile; the package test binary cannot link GRDB's sqlite3 on Linux, so CI runs
    them for real.
  • compileFullDebugKotlin, swift build, doc_comment_lint, i18n_audit --ci all clean.
  • Strand/Data/IntelligenceEngine.swift is app-target Swift that no default CI job builds —
    wants an app-build dispatch before merge.
  • Not validated against a real night. Every number above comes from synthetic sinusoids with
    known truth. They establish how much of a window the detector claims; they cannot establish that
    the staging inside a claimed night is accurate.

… median

Wires hrOnlySessions into the day scan for a strap with no motion, and fixes a real flaw in the
merged spine that writing the tests exposed.

THE ANCHOR. hrOnlySleepRuns took its band from hrBaseline, the window MEDIAN, on the reasoning that
reusing the motion path's definition was parity. That was wrong, and wrong by arithmetic rather than
by tuning: a median splits the samples in half by definition, so a band of median * 1.05 admits
strictly more than half of ANY window. Measured on a realistic 24 h - 16 h awake at 74-96, 8 h night
at 59-70 - it called 14.4 hours sleep against a truth of 8. hrSleepBandMult is correct where it was
designed to be used, as a CONFIRMATION gate on a run gravity stillness already found, where being
permissive is deliberate. A confirmation threshold and a detection threshold are not the same
instrument.

Now anchored on hrOnlyBaseline, the tenth percentile by nearest rank, which sits in the night's
trough and cannot admit half a window however the day is shaped. Chosen from a sweep over anchor x
multiplier against windows with known truth, taking the conservative corner because the two failure
directions are not symmetric: over-detection puts a wrong Rest on screen, under-detection leaves "No
data", which is the state this improves on. p10 x 1.05 measures 8.1 h against a truth of 8 on a
field-shaped day and under-reads a 54 h two-night window rather than over-reading it.

MERGE THE RUNS. A run boundary is a threshold crossing and a sleeping heart rate oscillates across
the band all night, so the spine returned the night's minutes shredded into sub-mergeMin fragments,
every one of which then failed the minimum-duration gate - 8 h of detected sleep yielding zero
sessions. mergePeriods first, exactly as the motion path does.

DISPLAY-ONLY, ENFORCED STRUCTURALLY. An HR-only night carries NULL restingHR and avgHRV, and the
AnalyticsEngine enrichment that would refill them now skips it. Those two values are what Charge and
the baselines fold in, and a baseline is the one thing a false positive cannot be unwound from.
Withholding them is structural; a downstream filter would be one forgotten call site from failing
open. The Swift enrichment also now names hrOnly when it rebuilds the struct - unlike Kotlin's copy,
that rebuild drops a new field by default.

A 4.0 IS UNTOUCHED. The fallback sits behind grav.size < 2 and only when the device persisted no
hypnogram of its own, so a strap that banks motion keeps the motion spine - a working detector must
not be replaced by a weaker one. Pinned by a source-reading test.

Two comments corrected that this change falsified: the no-motion diagnostic no longer claims the
stager has no HR-only fallback, and the Apple day-cache no longer justifies itself on "a WHOOP
always streams gravity".

18 Kotlin tests, 0 failures. Swift twins compile; the package test binary cannot link GRDB's sqlite3
on Linux, so CI runs them for real.

Refs #1801.
Re-review, and the finding is the exact failure this PR claimed to have designed out.

The display-only guarantee rested on an HR-only session carrying NULL restingHR/avgHRV. That only
protects an aggregate that READS those fields, and two of the day's physiological aggregates do not:

  - the deep-window HRV pool re-derives RMSSD from `rr` over each session's own stages;
  - the SDNN index filters `rr` to the matched session windows.

An HR-only night HAS stages, so both would have folded one in — and from there into Charge and the
HRV baseline, which is the one thing a false positive cannot be unwound from. deepHrvWindow is a user
setting (UnitPrefs.hrvWindow == DEEP_SLEEP), so that path is reachable rather than theoretical.

Fixed by naming the set once — physiologySessions = matched.filter { !it.hrOnly } — and routing all
four aggregates through it, rather than adding a filter at each site. The scattered version is what
produced this bug: I wrote "a downstream filter would be one forgotten call site away from failing
open" and then forgot two.

`matched` keeps its legitimate uses in analyzeDay — duration, stage totals, Rest, the onset — which
is why the guard checks the physiological region rather than banning the name.

Guarded by a source-reading test, deliberately rather than a behavioural one: the failure this
protects against is a FUTURE aggregate reaching for `matched` out of habit, and a behavioural test on
one path would not catch that. 20 Kotlin tests, 0 failures.
…e percentile twin

Re-review, two findings.

WakeMotionRefinement rebuilds a SleepSession field by field on Apple, so it dropped hrOnly by
default; the Kotlin twin is a `copy` and preserved it for free. Refinement only ever sees
motion-detected sessions today, so an HR-only night never reaches it — but the two platforms would
have disagreed silently the moment it did, and a flag that decides whether a night can touch the
baselines is the worst one to lose quietly. This is the second time in this PR that Swift's explicit
init dropped the field where Kotlin's copy kept it; both sites now name it.

The percentile index is now pinned against the Swift twin's own stdout across the sizes where a
nearest-rank rule can disagree: n below 1/p, and either side of an exact boundary (1, 2, 3, 7, 10,
11, 99, 100, 101, 1000). Both platforms truncate toward zero, so they agree — but that is now
asserted rather than reasoned, since a rounding rule is exactly the kind of thing that reads
identical and diverges.

21 Kotlin tests, 0 failures.
app-build caught what nothing else could. Strand/Data/IntelligenceEngine.swift is the day scan and
lives OUTSIDE the StrandAnalytics package, so an internal entry point is invisible to it:

  Strand/Data/IntelligenceEngine.swift:1182:39:
  error: 'hrOnlySessions' is inaccessible due to 'internal' protection level

Both app targets failed on it while every default CI job stayed green, because none of them builds
the app target — `swift build` compiles the package, where internal resolves fine, and that is what
I had been checking locally.

Only the entry point is promoted. The spine, the anchor and hrOnlyBaseline stay internal: the tests
reach them with @testable, and nothing outside this package should be assembling its own spine.
@ryanbr
ryanbr merged commit db1b81f into main Sep 2, 2026
17 checks passed
@ryanbr
ryanbr deleted the feat/1801-wire-hr-only-spine branch September 2, 2026 08:13
ryanbr added a commit that referenced this pull request Sep 2, 2026
Staging bump, so all three numbers move together: MARKETING_VERSION and versionName to 11.1.0, and
the build numbers past the staging high-water — versionCode 411 -> 412 and CURRENT_PROJECT_VERSION
292 -> 293. 411 was taken by the 11.0.0 release, so 412 is the first free code a side-installed
staging build can carry without the installer refusing it as a downgrade.

fork-testing-build.yml reads MARKETING_VERSION out of project.yml for the asset names and the
release title, so the testing artefacts will read v11.1.0.

Carries the HR-only sleep spine (#1805) and its wiring (#1806): a strap that streams heart rate but
banks no motion can now stage a night from HR alone, marked lower-confidence and kept out of Charge
and the baselines. No schema change, so no migration.
ryanbr added a commit that referenced this pull request Sep 2, 2026
Perf audit of this PR, second finding. The epoch count built a set over EVERY sample -
mapTo(HashSet()) on Kotlin, Set(hrS.map{}) on Swift, the latter allocating a full intermediate array
as well - to arrive at a few thousand distinct keys. That is ~160k boxed values per scored day across
the 21-day rescore, and it is precisely the allocation shape #707 names as the heap-churn source
under the OOM.

hrS is already sorted by timestamp, so the bucket key is non-decreasing and a running comparison
counts them with no allocation at all. Pinned by a test, because the zero-allocation version trades
the set for that ordering assumption: unsorted input would silently over-count rather than fail.

The rest of the audit came back clean, and the parts that look expensive are not this PR's. The bpm
boxing in the percentile sort and the epoch HashMap inside hrOnlySleepRuns both predate it - they
arrived with the spine in #1805/#1806 - and this change does not add to either. Worth a separate look
if the spine ever runs on every day rather than only motionless ones; not worth widening a
diagnostic PR to chase.

26 HR-only tests, 0 failures; swift builds; doc_comment_lint clean.
ryanbr added a commit that referenced this pull request Sep 2, 2026
…fallback

Re-review. Removing the owner exclusion does not only unblock the 5/MG it was written for - it makes
the branch reachable for ANY owner, including a WHOOP 4.0 day that banked nothing at all. I had told
the maintainer a 4.0 was untouched "because grav.size < 2 is false", which is true of a normal 4.0
day and not true of an empty one.

Low risk, and not zero: such a day has too little of anything to clear minSleepMin, but "too little"
is not "none", and the night it could produce is display-only and marked hrOnly like every other. The
comment now says so at the branch rather than leaving the next reader to work out that the gate
widened.

Also verified this pass, and sound: the guard test splits the expression on "else ->", and there is
exactly one of those, one importedDeviceId, none after the split. The middle branch returning
emptyList() for an import-namespace day with stored sessions preserves what both #804 and pre-#1806
did - that path has always yielded nothing there, so nothing moved. And a normal WHOOP day never
reaches the stored lookup at all, since grav.size >= 2 short-circuits it, so the added query lands
only on days that were already scoring blank.
ryanbr added a commit that referenced this pull request Sep 2, 2026
…eep WHOOP out (#1813)

The spine had never once run. The gate line from #1812 said so on the first build that carried it:

  [sleep] hr-only gate attempted=false reason=imported-owner grav=0 stored=0

on a day holding 165,980 HR rows - skipped before any heart rate was looked at. Neither of the two
failures the funnel was built to separate; it never got that far.

#1806 took its condition verbatim from #804 Fix A, whose comment says it exists "so WHOOP straps and
the my-whoop import namespace are untouched". #804 is about Oura RINGS and deliberately excludes
WHOOP. The HR-only fallback is FOR a WHOOP strap, so it inherited a condition whose entire purpose was
to exclude the case it was built for. resolveDayOwner returns importedDeviceId whenever the owner
source is absent or the candidates collapse to it, which on a live 5/MG install is every day - so the
p10 anchor tuned in #1806 was never reached either. Two owner concepts disagree, which is why older
logs could not show this: the [universal] dayOwner line reports the strap id while resolveDayOwner
returns the imported one.

Split by question rather than by gate. #804's stored-hypnogram branch keeps the owner exclusion. The
heart-rate fallback drops it. Between them a new branch leaves any day alone that already has stored
sessions, whoever owns it, because inferring a night from heart rate when the device recorded a real
one replaces better evidence with worse. The stored lookup now runs for every no-motion day, so
"nothing else knows about this night" is checked rather than assumed - one indexed read, and only on
days that were already scoring blank, since a normal WHOOP day short-circuits at grav.size >= 2.

Re-review recorded what dropping the exclusion widens: the branch is now reachable for ANY owner,
including a WHOOP 4.0 day that banked nothing at all. A normal 4.0 day never reaches the gate, and an
empty one has too little to clear minSleepMin - but "too little" is not "none", and I had described a
4.0 as untouched more broadly than was true.

Both platforms. 26 HR-only tests, 0 failures. The Apple half is app-target Swift that no default CI
job builds; the testing build's iOS and macOS legs are what compile it.

Refs #1801.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant